Skip to content

chore: code-review cleanup on today's shipped PRs (dead code + better errors) - #1141

Merged
HongmingWang-Rabbit merged 1 commit into
mainfrom
fix/review-cleanup-2026-04-20
Apr 20, 2026
Merged

HongmingWang-Rabbit merged 1 commit into
mainfrom
fix/review-cleanup-2026-04-20

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

What

Three nits from post-merge review of #1119 / #1133:

  1. Dead code — `ContextMenu.tsx` still imported `removeNode` from the store after fix(canvas): delete workspace dialog race with context menu close #1133 moved the delete confirm to Canvas. Removed. Same for the unused `removeNode` mock entry in `ContextMenu.keyboard.test.tsx`.

  2. Silent preflight YAML parse failure — `missingRequiredEnv` returned `nil` on parse error (safe default, the in-container preflight owns schema), but left no trace for ops. Added a WARN log so malformed templates are visible.

  3. User-facing error formatting — `formatMissingEnvError` used `%q` on a slice → `["A" "B"]`. Ugly and looks like a Go literal. Switched to `strings.Join(missing, ", ")`. Test updated.

Why direct-merge to main

User authorised direct-merge on review cleanup; zero behavioural impact beyond one log line.

Test plan

  • `go build ./...` clean
  • Preflight tests: `go test ./internal/handlers/... -run TestMissingRequiredEnv|TestFormatMissingEnvError` pass
  • Canvas tests: `npx vitest run src/components/tests/ContextMenu.keyboard.test.tsx` — 15/15 pass

Three nits identified during post-merge review of #1119, #1133:

1. ContextMenu.tsx imported `removeNode` from the canvas store but
   stopped using it when the delete-confirm flow moved to Canvas in
   #1133. Also removed the now-unused mock entry in the keyboard
   test so the test inventory matches the real call list.

2. Preflight's YAML parse failure was a silent pass — defensible since
   the in-container preflight owns the schema, but invisible to ops if
   a template ships malformed YAML. Log at WARN so the signal surfaces
   without blocking the provision.

3. formatMissingEnvError rendered its slice via %q, producing
   `["A" "B"]` which is Go-literal-looking and ugly in a user-facing
   error. Join with ", " instead. Test updated to assert the new
   format.

No behavioural changes beyond the log line; fixes are review nits, not
bug fixes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@HongmingWang-Rabbit
HongmingWang-Rabbit merged commit 8e0f8e9 into main Apr 20, 2026
5 checks passed
molecule-ai Bot pushed a commit that referenced this pull request Apr 21, 2026
chore: code-review cleanup on today's shipped PRs (dead code + better errors)
@molecule-ai
molecule-ai Bot deleted the fix/review-cleanup-2026-04-20 branch May 20, 2026 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant